Fix pom involving maven-archetype and opentelemetry - #38518
Conversation
* Fix opentelemetry version * Fix target platform version to be 11
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses dependency resolution issues involving OpenTelemetry and Maven archetypes. By aligning OpenTelemetry dependencies with the project's BOM and upgrading the default Java target platform, the changes ensure better compatibility and correct transitive dependency resolution for users of the Beam Maven archetypes. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates OpenTelemetry dependency management by switching BOM references and adding an explicit version for the context module. It also upgrades the default Java target platform from 1.8 to 11 in Maven archetypes. Feedback indicates that explicit versions should also be added to other OpenTelemetry artifacts (api, sdk, exporter-otlp, and autoconfigure) to ensure correct propagation to generated POMs. Furthermore, the Java version upgrade is a breaking change that needs to be documented.
| netty_transport : "io.netty:netty-transport:$netty_version", | ||
| netty_transport_native_epoll : "io.netty:netty-transport-native-epoll:$netty_version", | ||
| opentelemetry_api : "io.opentelemetry:opentelemetry-api", // google_cloud_platform_libraries_bom sets version | ||
| opentelemetry_api : "io.opentelemetry:opentelemetry-api", // opentelemetry-bom sets version |
There was a problem hiding this comment.
The issue #30505 specifically mentions that opentelemetry-api and opentelemetry-sdk are missing versions in the generated POMs. While you've added an explicit version for opentelemetry_context at line 864, opentelemetry_api still lacks one. It should likely be updated to include $opentelemetry_version to ensure it is correctly propagated to the archetype POMs.
opentelemetry_api : "io.opentelemetry:opentelemetry-api:$opentelemetry_version", // opentelemetry-bom sets version
| opentelemetry_sdk : "io.opentelemetry:opentelemetry-sdk", // opentelemetry-bom sets version | ||
| opentelemetry_exporter_otlp : "io.opentelemetry:opentelemetry-exporter-otlp", // opentelemetry-bom sets version | ||
| opentelemetry_extension_autoconfigure : "io.opentelemetry:opentelemetry-sdk-extension-autoconfigure", // opentelemetry-bom sets version |
There was a problem hiding this comment.
Similar to opentelemetry_api, these artifacts (sdk, exporter-otlp, and autoconfigure) should also have explicit versions to resolve the issues reported in #30505 regarding missing versions in published POMs used by archetypes.
opentelemetry_sdk : "io.opentelemetry:opentelemetry-sdk:$opentelemetry_version", // opentelemetry-bom sets version
opentelemetry_exporter_otlp : "io.opentelemetry:opentelemetry-exporter-otlp:$opentelemetry_version", // opentelemetry-bom sets version
opentelemetry_extension_autoconfigure : "io.opentelemetry:opentelemetry-sdk-extension-autoconfigure:$opentelemetry_version", // opentelemetry-bom sets version
There was a problem hiding this comment.
These modules are currently only used in java-core, where enforcePlatform(opentelemetry-bom) is correctly configured. For other places, only opentelemetry_context is used. Adding a version in beampluginmodule will force the Beam modules pin on this version.
| <requiredProperties> | ||
| <requiredProperty key="targetPlatform"> | ||
| <defaultValue>1.8</defaultValue> | ||
| <defaultValue>11</defaultValue> |
|
R: @aIbrahiim |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
|
LGTM from my side |
* Fix opentelemetry version * Fix target platform version to be 11
[Release-2.74] Cherry-pick #38518 into release branch
* Fix opentelemetry version * Fix target platform version to be 11
Please add a meaningful description for your change here
Fix #30505
Tested:
./gradlew :runners:google-cloud-dataflow-java:publishToMavenLocal -Ppublishingthenmvn dependency:tree -Pdataflow-runnerfrom maven archetype project can show transitive dependencies of this moduleThank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.